Skip to content

rrdp: discard stats #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

AnthoineB
Copy link
Member

Add an extended stats structure to get discards' fields and compute new statistics:

  • discard_reqs_submitted,
  • discard_reqs_completed,
  • discard_sectors,
  • discard_total_ticks.

@AnthoineB AnthoineB requested a review from gthvn1 March 24, 2025 14:09
@AnthoineB AnthoineB self-assigned this Mar 24, 2025
@coveralls
Copy link

coveralls commented Mar 24, 2025

Pull Request Test Coverage Report for Build 14044455376

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 77.703%

Totals Coverage Status
Change from base Build 14026377529: 0.0%
Covered Lines: 3356
Relevant Lines: 4319

💛 - Coveralls

@AnthoineB AnthoineB force-pushed the discard-stats branch 2 times, most recently from 6656ca0 to 8cc3781 Compare March 24, 2025 14:27
@@ -42,6 +42,28 @@ type stats = {
}
[@@little_endian]]

[%%cstruct
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need a V2 I think. You can put it in the stats struct and just have wrappers that checks version when accessing these specific fields.

Copy link

@gthvn1 gthvn1 Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But as discussed the problem is now we are not able to know the size of v1 vs the size of v2 that is used by "copy"... So it looks like a v2 is needed. To avoid generating unneeded function you can do something like:

type stats_v2 = {
    _version: uint32_t
  ; _pad: uint32_t
  ; _oo_reqs: uint64_t
  ; _read_reqs_submitted: uint64_t
  ; _read_reqs_completed: uint64_t
  ; _read_sectors: uint64_t
  ; _read_total_ticks: uint64_t
  ; _write_reqs_submitted: uint64_t
  ; _write_reqs_completed: uint64_t
  ; _write_sectors: uint64_t
  ; _write_total_ticks: uint64_t
  ; _io_errors: uint64_t
  ; _flags: uint64_t
  ; discard_reqs_submitted: uint64_t
  ; discard_reqs_completed: uint64_t
  ; discard_sectors: uint64_t
  ; discard_total_ticks: uint64_t
}
[@@little_endian]]

Not sure if it is the correct approach but you will have both sizes and new successors accessors to V2 fields...

@AnthoineB AnthoineB force-pushed the discard-stats branch 2 times, most recently from 565cf1c to 09746c2 Compare March 24, 2025 19:42
Signed-off-by: Anthoine Bourgeois <[email protected]>
@gthvn1
Copy link

gthvn1 commented Mar 25, 2025

It looks good to me. At least I don't see a better way to do that. Also as it manages the blktap V2 struct I think that it can be proposed upstream... maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants